Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symphony PRs in FDC3 2.1 candidates #882

Conversation

symphony-jean-michael
Copy link
Contributor

CHANGELOG.md Outdated
* Added a `SendChatMessage` intent to be used when a user wants to send a message to an existing chat room. ([#794](https://github.com/finos/FDC3/pull/794))
* Added a context type representing a chat message (`fdc3.chat.message`). ([#794](https://github.com/finos/FDC3/pull/794))
* Added a context type representing a chat room (`fdc3.chat.room`). ([#794](https://github.com/finos/FDC3/pull/794))
* Added a chat `Message` type in order to describe messages with rich content and attachments
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@symphony-jean-michael each changelog entry needs to include a Github PR that introduced the change (e.g. addition or removal)

CHANGELOG.md Outdated
* Added a context type representing a chat message (`fdc3.chat.message`). ([#794](https://github.com/finos/FDC3/pull/794))
* Added a context type representing a chat room (`fdc3.chat.room`). ([#794](https://github.com/finos/FDC3/pull/794))
* Added a chat `Message` type in order to describe messages with rich content and attachments
* Added an `Action` type, encapsulating either a `Context` or the association of a `Context` with an `Intent` inside another object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as previous comment.

CHANGELOG.md Outdated
### Changed

* Updated definition of the `ChatInitSettings` context type to use the new `Message` type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as previous comment.

}
}

## See Also
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block of text is also included in the example section. Needs to be moved outside of the example.

Copy link
Contributor

@mistryvinay mistryvinay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments on this one @symphony-jean-michael, just need reviewing.

@symphony-jean-michael
Copy link
Contributor Author

Hi @mistryvinay
Thank you for your review. I updated the PR accordingly to your comments. Can you please have a second look ?
Thanks

Copy link
Contributor

@mistryvinay mistryvinay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.

@mistryvinay mistryvinay added the Context Data & Intents Contexts & Intents Discussion Group label Jan 3, 2023
@mistryvinay mistryvinay requested review from kriswest, a team and mistryvinay January 3, 2023 12:29
@mistryvinay mistryvinay changed the base branch from master to context-data-and-intents-consolidated-update-branch-2.1 March 22, 2023 13:21
@symphony-jean-michael
Copy link
Contributor Author

Hi @mistryvinay
I have updated an example in this PR to show a real case of how we will use it in Symphony. But it seems it removed your approval. Could you please re-approved it ?
I hope this PR can be merged soon into 2.1 :)
Thanks

Copy link
Contributor

@mistryvinay mistryvinay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kriswest kriswest force-pushed the context-data-and-intents-consolidated-update-branch-2.1 branch from 06ff641 to ab3660d Compare March 24, 2023 18:39
@mistryvinay mistryvinay merged commit 0803274 into finos:context-data-and-intents-consolidated-update-branch-2.1 Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Context Data & Intents Contexts & Intents Discussion Group
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants